-
Notifications
You must be signed in to change notification settings - Fork 30
Fixes lint warnings #588
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixes lint warnings #588
Conversation
Refer: 1. ros-frontend/src/Components/InstanceTypes/SuggestedInstanceTypesTable.js 42:8 warning React Hook useEffect has a missing dependency: 'dispatch'. Either include it or remove the dependency array - react-hooks/exhaustive-deps 2. ros-frontend/src/Routes/RosSuggestedInstanceTypes/RosSuggestedInstanceTypes.js 28:8 warning React Hook useEffect has a missing dependency: 'dispatch'. Either include it or remove the dependency array - react-hooks/exhaustive-deps 65:8 warning React Hook useEffect has missing dependencies: 'hideGlobalFilter' and 'updateDocumentTitle'. Either include them or remove the dependency array - react-hooks/exhaustive-deps
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR resolves react-hooks/exhaustive-deps lint warnings by explicitly adding missing dependencies to several useEffect hooks across the SuggestedInstanceTypesTable and RosSuggestedInstanceTypes components. Class diagram for updated useEffect dependencies in SuggestedInstanceTypesTable and RosSuggestedInstanceTypesclassDiagram
class SuggestedInstanceTypesTable {
+dispatch
+page
+perPage
+activeSortDirection
+activeSortColumnKey
+instanceTypeName
useEffect(loadSuggestedInstanceTypes) // dependencies: dispatch, page, perPage, activeSortDirection, activeSortColumnKey, instanceTypeName
}
class RosSuggestedInstanceTypes {
+dispatch
+updateDocumentTitle
+hideGlobalFilter
useEffect(loadIsConfiguredInfo) // dependencies: dispatch
useEffect(updateDocumentTitle, hideGlobalFilter) // dependencies: updateDocumentTitle, hideGlobalFilter
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes - here's some feedback:
- Ensure that updateDocumentTitle and hideGlobalFilter are stable (e.g., memoized via useCallback) so adding them to the dependency arrays doesn’t cause unintended effect re-runs.
- Consider extracting the document title and global filter side-effects into a custom hook to DRY up the repeated useEffect logic across components.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Ensure that updateDocumentTitle and hideGlobalFilter are stable (e.g., memoized via useCallback) so adding them to the dependency arrays doesn’t cause unintended effect re-runs.
- Consider extracting the document title and global filter side-effects into a custom hook to DRY up the repeated useEffect logic across components.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Refer:
PR Title 💥
Please title this PR with a summary of the change, along with the JIRA card number.
Suggested formats:
Feel free to remove this section from PR description once done.
Why do we need this change? 💭
Please include the context of this change here.
Documentation requires update? 📝
Security Checklist 🔒
Upon raising this PR please go through RedHatInsights/secure-coding-checklist
💂♂️ Checklist 🎯
Additional 📣
Feel free to add any other relevant details such as links, notes, screenshots, here.
Summary by Sourcery
Ensure exhaustive-deps lint compliance by adding missing dependencies to useEffect hooks in RosSuggestedInstanceTypes and SuggestedInstanceTypesTable
Enhancements: